Skip to content

Claim the on-screen blocks instead of driving core's required channel - #420

Merged
nbenn merged 7 commits into
mainfrom
417-unified-demand
Sep 29, 2026
Merged

nbenn merged 7 commits into
mainfrom
417-unified-demand

Conversation

@nbenn

@nbenn nbenn commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Core has retired the per-block required channel (Fold front-end demand into the one multi-owner claim set blockr.core#337), so visibility$required[[id]](…) fails with attempt to apply non-function. The dock now states its evaluation demand as the blocks it holds eager, under its own owner label.
  • The board callback makes the board lazy by returning eager(owner, opening), with the active view's front panels as the opening set, which core seeds as it runs the callbacks, before the first flush decides what to construct. Nothing travels through update to open the set: eager_holder() starts from the declared set and sends only a change, one eager payload with a set delta per view switch.
  • The settled _state echo drives two writers of the single update channel, the geometry mirror and the eager holder, so both fold into whatever is pending (fold_update()) rather than one replacing the other.
  • A card the dock has built but is not showing gets no construction demand. The construct component builds every id it names in the flush that applies it, where the retired channel's FALSE state was paced; fronting a tab holds its block eager instead.

Test changes

The #413 narrow-viewport test asserted commit_count == 0 to show that the echo writes nothing back. That export counts every update payload, and the dock's eager payloads are update payloads: a narrow load sends two, both eager updates according to core's debug log, with no grid commit among them. A narrow view also reports its stored grid as its live one, so roundtrip_stable holds trivially and cannot stand in. The test now reads the stored grid through a new stored_grids test export and compares it with the authored one; wiring the mirror in narrow mode makes it fail.

Two new tests drive core's own board server with the dock's callback. One checks that the declared owner becomes the gate, and its blocks that owner's eager set, before any flush, with nothing sent to get there. The other sends the dock's payload and checks that core's eager set moved. Core passes unknown top-level keys through for board subclasses, so a payload under a name core does not read is accepted and changes nothing, and the unit tests cannot see that because they check the payload against names they spell out themselves. With the payload and those names both left at sustain, only this test fails. The fake visibility bundle the other tests use is what hid the original breakage from them.

The #377 test for a bare required write reading back as a built card is dropped rather than ported: demand left the visibility bundle, so the dock is the only writer of visible left.

Fixes #417

@codecov

codecov Bot commented Aug 19, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
R/block-ui.R 98.44% <100.00%> (-0.05%) ⬇️
R/board-server.R 86.55% <100.00%> (+0.41%) ⬆️
R/utils-serve.R 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nbenn

nbenn commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Carries into https://github.com/orgs/BristolMyersSquibb/discussions/489 after BristolMyersSquibb/blockr.core#337's rework: dock declares its opening screen through an initial_block_ids.dock_board() method instead of writing visibility$gate(). Keep this PR to claims. Ordered construction requests, freezing through update, and dropping the paint report and the build ledger are #486.

@nbenn

nbenn commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Revised request, replacing my comment above, to follow BristolMyersSquibb/blockr.core#337's rework under https://github.com/orgs/BristolMyersSquibb/discussions/489:

  1. Declare dock's callback as the gating front-end when it is registered, with the active view's blocks as its opening claim, in whatever shape Fold front-end demand into the one multi-owner claim set blockr.core#337 settles on, instead of writing visibility$gate(). Core seeds that claim before the first flush.
  2. Drop the forced opening claim, sent "regardless, empty active view included": it exists for core's gate_claimed latch, which goes.
  3. Rebase onto main, which is 67 commits ahead and conflicts, and re-pin to the reworked core PR.

The block_claim() set per view switch, fold_update(), mark_cards_hidden() and sending no construct for off-screen cards stay as they are. Ordered construction requests, freezing through update, and dropping the paint report and the build ledger are #486, not this PR.

Core retired the per-block `required` channel, so the dock declares
itself the gating front-end and holds what it shows as a `sustain`
claim under that owner label. The settled layout echo now drives two
writers of the single update channel, so both fold into the pending
payload rather than replacing it.
lintr 3.4.0's assignment_linter rejects a `<<-` whose target lives only
inside the closure, which is exactly the shape of the sent-set cache.
Core now takes the gating front-end's declaration from what its callback
returns, and seeds the opening claim before the first flush. The dock
returns gate_claim() with the active view's front panels rather than
writing visibility$gate(), and no longer sends that opening claim through
update: block_claim() starts from it and sends only a change.
The update tally now counts the dock's claims, and a narrow view reports
its stored grid as its live one, so neither can show that the echo wrote
nothing back. Read the stored grid through a test export instead.
blockr.ui's bare blockr.core remote conflicts with this branch's pin, so
dependency resolution fails. Its 321-unified-demand branch pins the same
core branch; drop both pins once blockr.core#337 merges.
@nbenn
nbenn force-pushed the 417-unified-demand branch from e68907c to 1281bd5 Compare September 28, 2026 16:04
@nbenn

nbenn commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Done on all three:

  1. The callback returns gate_claim(owner, opening) next to its plugin values, with the active view's front panels as the opening claim, and the visibility$gate() write is gone.
  2. The forced opening claim is gone too: block_claim() starts from the declared set and sends only a change.
  3. Rebased onto main and re-pinned to the reworked core#337. The rebase brought in a blockr.ui dependency whose bare blockr.core remote conflicts with that pin, so a throwaway 321-unified-demand branch on blockr.ui pins the same core branch, and this PR pins that.

One test outside that list had to change: the #413 narrow-viewport test asserted commit_count == 0, which the dock's claims now tick. The description has the details.

@nbenn

nbenn commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

BristolMyersSquibb/blockr.core#337 renamed the demand vocabulary this PR adopts, so this PR needs to follow before #337 can merge: #337's deps block points the merge queue's revdep check at this branch. Nothing was released under the old names, so there is no deprecation path.

Before After
gate_claim(owner, blocks) eager(owner, blocks)
payload component sustain eager, with the same set / add / rm deltas
declaration class "gate_claim" "eager_blocks"

On this branch that means:

  • The declaration in board_server_callback() (R/board-server.R, gate = gate_claim(owner, opening)) becomes eager(owner, opening). Missing this one is loud: the callback errors at startup.
  • The runtime payload passed to fold_update() (R/board-server.R, list(sustain = ...)) becomes list(eager = ...). Missing this one is silent. Core passes unknown top-level keys through for board subclasses, so a sustain payload now reports ok = TRUE in board$last_update and changes nothing: a view switch stops changing what is evaluated, without any error.
  • The tests read the payload by name (tests/testthat/helpers.R, update()[["sustain"]]; tests/testthat/test-board-server.R, the c("views", "sustain") names and the expected list(sustain = ...) payload) and check the declaration's class (expect_s3_class(res$gate, "gate_claim")).

The comments describing a sustain claim or gate_claim() (R/block-ui.R, R/board-server.R, tests/testthat/helpers.R) would read better in the new terms too: core's docs now say a board is eager by default and a front-end makes it lazy by returning eager(owner, blocks). Core finds the declaration by its class, not by the name of the list element carrying it, so gate = can stay or become eager =.

The rename in blockr.core#337 turned gate_claim() into eager(), the
`sustain` payload component into `eager` and the declaration's class
into "eager_blocks", and retired "claim" as a word for any of it. The
callback now returns eager(owner, opening), under `eager` rather than
`gate`, and the dock sends its on-screen blocks as an `eager` set. Its
own wording follows: block_claim() is eager_holder(), and the tests
read core's rv$eager_blocks() where they read rv$claims().

A payload under a name core does not read is accepted and changes
nothing, and the unit tests check the payload against names they spell
out themselves, so a rename missed in both would pass them. A new test
sends the dock's payload through core's board server and checks that
core's eager set moved.

The build-ledger comment called core's "Show code" a peer owner holding
blocks. It asks for construction, which names no owner.
@nbenn

nbenn commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

Applied. Beyond the list:

  • The integration test read core's rv$claims(), which is now rv$eager_blocks().
  • Core retired "claim" altogether, so the dock's own uses follow: block_claim() is eager_holder(), the test helper claimed_blocks() is held_eager(), and the declaration rides under eager =.
  • A new test sends the dock's payload through core's board server and checks that core's eager set moved. The unit tests check the payload against names they spell out themselves, so with the payload and those names both left at sustain, this is the only test that fails.
  • The ledger comment in R/block-ui.R called "Show code" a peer owner holding blocks. It asks for construct, which names no owner, and the comment now says so.

Core's main now carries eager(), and the branch the pin named was
deleted with the merge, so blockr.core resolves from main again. That
also ends the conflict that pinned blockr.ui: its bare blockr.core
remote matches the dock's once more.
@nbenn
nbenn marked this pull request as ready for review September 29, 2026 10:36
@nbenn
nbenn added this pull request to the merge queue Sep 29, 2026
Merged via the queue into main with commit aca6706 Sep 29, 2026
10 checks passed
@nbenn
nbenn deleted the 417-unified-demand branch September 29, 2026 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adopt core's unified evaluation-demand claim set

1 participant